Fix Atoms.cluster_from_file() in io.feff.inputs giving wrong number of atoms
#3426
Conversation
Signed-off-by: KaifengZheng <[email protected]>
Signed-off-by: KaifengZheng <[email protected]>
Signed-off-by: KaifengZheng <[email protected]>
This reverts commit 8112068.
…checks initialize PymatgenTest.TEST_STRUCTURES as dict[Path, None]
Signed-off-by: KaifengZheng <[email protected]>
Signed-off-by: KaifengZheng <[email protected]>
Signed-off-by: KaifengZheng <[email protected]>
Signed-off-by: KaifengZheng <[email protected]>
pymatgen/io/feff/inputs.py
Outdated
| coords = [] | ||
| symbols = [] | ||
| for line in lines: | ||
| if line: | ||
| if line and line[0].__contains__("*"): |
There was a problem hiding this comment.
| if line and line[0].__contains__("*"): | |
| if line and line.startswith("*"): |
tests/io/feff/test_inputs.py
Outdated
| def test_atom_num(self): | ||
| atoms = Atoms.cluster_from_file("ATOMS_test.inp") | ||
| num_atom = 37 | ||
| assert len(atoms) != num_atom |
There was a problem hiding this comment.
why not test for the expected number of atoms?
There was a problem hiding this comment.
Hi Janosh,
The expected number of atoms is 37; how should I specify this value?
There was a problem hiding this comment.
You're missing one level of indentation to make it a method of the test class.
There was a problem hiding this comment.
Let me fix it. sorry to confuse you. I want it to pass when the expected number of atoms is equal to num_atom and throw an error when the numbers are not equal.
tests/files/ATOMS_test.inp
Outdated
There was a problem hiding this comment.
We have 6 or more existing .inp files. Can you reuse those for testing?
There was a problem hiding this comment.
Every updated feff.inp file has the same format. My input file deletes the * lines under ATOMS, which can be used for the test.
Signed-off-by: KaifengZheng <[email protected]>
Signed-off-by: KaifengZheng <[email protected]>
Signed-off-by: KaifengZheng <[email protected]>
tests/io/feff/test_inputs.py
Outdated
| def test_atom_num(self): | ||
| atoms = Atoms.cluster_from_file("ATOMS_test.inp") | ||
| num_atom = 37 | ||
| assert len(atoms) != num_atom |
There was a problem hiding this comment.
You're missing one level of indentation to make it a method of the test class.
Signed-off-by: KaifengZheng <[email protected]>
Signed-off-by: KaifengZheng <[email protected]>
Signed-off-by: KaifengZheng <[email protected]>
Signed-off-by: KaifengZheng <[email protected]>
Signed-off-by: KaifengZheng <[email protected]>
for line in lines:
> if line and not line.startswith("*"):
janosh
changed the title
Atoms.cluster_from_file() in io.feff.inputs giving wrong number of atoms
Summary
Atoms.cluster_from_file() generates a cluster with a wrong number of atoms because it skips wrong lines when the format of feff input file in the Atom parts is different.
Major changes:
Checklist
ruff.mypy.duecredit@due.dcitedecorators to reference relevant papers by DOI (example)Tip: Install
pre-commithooks to auto-check types and linting before every commit: